chore: bump minimum Go from 1.24 to 1.25#354
Conversation
- Remove GOEXPERIMENT=synctest (GA in Go 1.25) - Update SDK pinning from go1.24.13 to go1.25.8 - Add tidy/fmt to QUALITY_TARGETS for consistent SDK usage - Bump go-version in all CI workflows to 1.25 - Replace deprecated goparser.ParseDir with os.ReadDir + ParseFile - Update all docs/examples to require Go 1.25+ - Preserve "Go 1.24+" attribution on b.Loop() benchmark references Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughUpgrades the repository and examples from Go 1.24 → 1.25 (various docs, go.mod files, CI workflows, Makefile). Removes workflow/workspace Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can suggest fixes for GitHub Check annotations.Configure the |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/doctest/doc_api_sync_test.go (1)
231-266: 🧹 Nitpick | 🔵 TrivialRefactor addresses SA1019 correctly; minor variable shadowing to consider.
The per-file parsing approach properly replaces the deprecated
goparser.ParseDir. However,nameat line 257 shadows the outernamevariable (line 236) which holds the filename. While this doesn't cause a functional bug since the filename isn't used after this point, it can trigger linter warnings and cause confusion.♻️ Suggested rename for clarity
case *ast.ValueSpec: - for _, name := range s.Names { - if name.IsExported() { - syms[name.Name] = true + for _, ident := range s.Names { + if ident.IsExported() { + syms[ident.Name] = true } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/doctest/doc_api_sync_test.go` around lines 231 - 266, The variable name used for the file name (assigned as name in the entries loop and used in ParseFile/error messages) is shadowed by the inner loop variable "name" in the ValueSpec loop; rename the inner loop variable (e.g., to ident or valName) in the for _, name := range s.Names loop so it no longer shadows the file-level name, keeping references to the file name (ParseFile and require.NoError message) unchanged; update any usages inside that inner loop to use the new identifier (e.g., ident.Name) instead..github/workflows/release.yml (1)
8-10:⚠️ Potential issue | 🟠 MajorReduce default workflow token scope to least privilege.
Setpermissions: contents: readat the top level. The currentcontents: writeis overly broad—actions/checkout@v6requires only read access, and goreleaser-action uses a custom PAT (secrets.HOMEBREW_TAP_TOKEN) that bypasses the default token's permissions entirely. Grant write only if truly required at the job or step level, following principle of least privilege (CWE-275).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 8 - 10, Top-level workflow permissions are too permissive: change the top-level permissions entry from contents: write to contents: read, and if any job or step truly needs write, grant contents: write only on that specific job/step; reference the top-level permissions block and the steps using actions/checkout@v6 and goreleaser-action (which should rely on secrets.HOMEBREW_TAP_TOKEN) to scope the write permission down to where the custom PAT is used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/workflows/collision-resolution/go.mod`:
- Line 3: Update the go directive in the module file to remove the
patch/toolchain patch version: replace the current `go` directive value `1.25.8`
with the language version `1.25` (i.e., change the `go` directive in the go.mod
to `go 1.25`) so the file uses the proper Go language version syntax rather than
a patched toolchain number; if you need to pin a specific toolchain, use a
separate `toolchain` directive instead.
In `@generator/compile_test.go`:
- Around line 90-96: Multiple test helpers create duplicate inline go.mod
content via the goModContent variable; centralize this into a single constant or
helper to prevent version drift. Replace repeated inline goModContent usages
with a shared constant (e.g., testGoModTemplate) or a helper function (e.g.,
WriteTestGoMod(outputDir, goVersion)) and update callers that currently set
goModContent and call os.WriteFile to use that helper/constant instead (refer to
the goModContent occurrences in compile_test.go). Ensure the helper writes the
same file permissions and retains the go version placeholder so version bumps
happen in one place.
In `@Makefile`:
- Around line 12-15: Add a Makefile preflight that fails fast when the
configured Go toolchain is missing or older than 1.25: in the block that
currently uses GO_SDK and QUALITY_TARGETS, add a check (using GO_SDK or `go
version` when GO_SDK is unset) to parse the Go major.minor version and abort
with a clear error if it is less than 1.25 (or if GO_SDK path like
$(HOME)/sdk/go1.25.8 is missing), so running the QUALITY_TARGETS (e.g.
check/lint/test) cannot proceed on an unsupported Go version; reference the
existing GO_SDK and QUALITY_TARGETS symbols when implementing the check so it
gates the same targets.
---
Outside diff comments:
In @.github/workflows/release.yml:
- Around line 8-10: Top-level workflow permissions are too permissive: change
the top-level permissions entry from contents: write to contents: read, and if
any job or step truly needs write, grant contents: write only on that specific
job/step; reference the top-level permissions block and the steps using
actions/checkout@v6 and goreleaser-action (which should rely on
secrets.HOMEBREW_TAP_TOKEN) to scope the write permission down to where the
custom PAT is used.
In `@internal/doctest/doc_api_sync_test.go`:
- Around line 231-266: The variable name used for the file name (assigned as
name in the entries loop and used in ParseFile/error messages) is shadowed by
the inner loop variable "name" in the ValueSpec loop; rename the inner loop
variable (e.g., to ident or valName) in the for _, name := range s.Names loop so
it no longer shadows the file-level name, keeping references to the file name
(ParseFile and require.NoError message) unchanged; update any usages inside that
inner loop to use the new identifier (e.g., ident.Name) instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 427f509a-3fee-41b2-b618-c66191af3135
⛔ Files ignored due to path filters (5)
examples/programmatic-api/builder/go.sumis excluded by!**/*.sumexamples/workflows/fixer-showcase/go.sumis excluded by!**/*.sumexamples/workflows/pipeline-compositions/go.sumis excluded by!**/*.sumexamples/workflows/validate-and-fix/go.sumis excluded by!**/*.sumgo.sumis excluded by!**/*.sum
📒 Files selected for processing (67)
.github/CONFIGURATION.md.github/copilot-instructions.md.github/workflows/benchmark.yml.github/workflows/go-race.yml.github/workflows/go.yml.github/workflows/golangci-lint.yml.github/workflows/release.ymlAGENTS.mdCLAUDE.mdCONTRIBUTORS.mdMakefileREADME.mdbenchmarks.mddocs/claude-code-plugin.mddocs/index.mddocs/mcp-server.mddocs/whitepaper.mdexamples/petstore/README.mdexamples/petstore/chi/go.modexamples/petstore/stdlib/go.modexamples/programmatic-api/builder/README.mdexamples/programmatic-api/builder/go.modexamples/quickstart/README.mdexamples/quickstart/go.modexamples/validation-pipeline/README.mdexamples/validation-pipeline/go.modexamples/walker/api-documentation/README.mdexamples/walker/api-documentation/go.modexamples/walker/api-statistics/README.mdexamples/walker/api-statistics/go.modexamples/walker/public-api-filter/README.mdexamples/walker/public-api-filter/go.modexamples/walker/reference-collector/README.mdexamples/walker/reference-collector/go.modexamples/walker/security-audit/README.mdexamples/walker/security-audit/go.modexamples/walker/vendor-extensions/README.mdexamples/walker/vendor-extensions/go.modexamples/workflows/breaking-change-detection/README.mdexamples/workflows/breaking-change-detection/go.modexamples/workflows/collision-resolution/README.mdexamples/workflows/collision-resolution/go.modexamples/workflows/fixer-showcase/README.mdexamples/workflows/fixer-showcase/go.modexamples/workflows/http-validation/README.mdexamples/workflows/http-validation/go.modexamples/workflows/multi-api-merge/README.mdexamples/workflows/multi-api-merge/go.modexamples/workflows/overlay-transformations/README.mdexamples/workflows/overlay-transformations/go.modexamples/workflows/pipeline-compositions/README.mdexamples/workflows/pipeline-compositions/go.modexamples/workflows/schema-deduplication/README.mdexamples/workflows/schema-deduplication/go.modexamples/workflows/schema-renaming/README.mdexamples/workflows/schema-renaming/go.modexamples/workflows/validate-and-fix/README.mdexamples/workflows/validate-and-fix/go.modexamples/workflows/version-conversion/README.mdexamples/workflows/version-conversion/go.modexamples/workflows/version-migration/README.mdexamples/workflows/version-migration/go.modgenerator/compile_test.gogo.modinternal/doctest/doc_api_sync_test.gointernal/doctest/doc_test.goplugin/CLAUDE.md
In Go 1.25, synctest.Run was renamed to synctest.Test(t, func(t *testing.T)). The old Run function only exists behind GOEXPERIMENT=synctest as a migration bridge and will be removed in Go 1.26. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #354 +/- ##
=======================================
Coverage 84.77% 84.77%
=======================================
Files 194 194
Lines 27246 27246
=======================================
Hits 23097 23097
Misses 2829 2829
Partials 1320 1320 🚀 New features to boost your workflow:
|
Summary
GOEXPERIMENT=synctestfrom Makefile and all CI workflows (testing/synctestis GA in Go 1.25)go1.24.13→go1.25.8, addtidy/fmttoQUALITY_TARGETSgo-versionfrom1.24→1.25in all 5 GitHub Actions workflowsgoparser.ParseDirwithos.ReadDir+goparser.ParseFileininternal/doctest/(deprecated in Go 1.25, SA1019)b.Loop()benchmark references (feature was introduced in 1.24)Go 1.25 improvements we get for free
go vetanalyzers (waitgroup,hostport) — both clean in this codebaseTest plan
make checkpasses (8501 tests, 0 errors)b.Loop()references preserved as "Go 1.24+"🤖 Generated with Claude Code